feat: load workload values as is #832
Conversation
j-zimnowoda
requested review from
dennisvankekem and
ferruhcihan
as code owners
|
|
||
| export function renderManifest(fileMap: FileMap, jsonPath: jsonpath.PathComponent[], data: Record<string, any>) { | ||
| //TODO remove this custom workaround for workloadValues | ||
| if (fileMap.kind === 'AplTeamWorkloadValues') { |
There was a problem hiding this comment.
Can we completely remove the AplTeamWorkloadValues kind, or do we still need it?
There was a problem hiding this comment.
We can but it may be complicated at the moment. This kind is only used for loading and does not enforce any data structure that is reflected in values file.
What benefits from removing it now do you see?
src/otomi-stack.ts
Outdated
| const filePath = getTeamWorkloadValuesFilePath(teamId, metadata.name) | ||
| await this.git.writeTextFile(filePath, data.spec.values || '{}') | ||
| const filePathValuesManaged = getTeamWorkloadValuesManagedFilePath(teamId, metadata.name) | ||
| await this.git.writeTextFile(filePathValuesManaged, '') |
There was a problem hiding this comment.
I am not sure how the managed file should behave but this will always override the managed file to an empty file. I am assuming that its not correct.
There was a problem hiding this comment.
Good point I did not realized that saveTeamWorkloadValues is also called by editAplWorkload and editWorkloadValues
| const teamId = workload.metadata.labels?.['apl.io/teamId'] | ||
|
|
||
| const filePath = getTeamWorkloadValuesFilePath(teamId, workloadName) | ||
| return merge(workload, { spec: { values: files[filePath] } }) |
There was a problem hiding this comment.
I don't think these values: files is used right? We will always know what the workload values files are based on the functions: getTeamWorkloadValuesManagedFilePath and getTeamWorkloadValuesFilePath
There was a problem hiding this comment.
yes, they are. At lease this is what APL Console expects. Otherwise you won't see values in the code editor.
| const fileMaps = getFileMaps(envDir) | ||
| const spec = {} | ||
| const spec = { | ||
| files: {}, |
There was a problem hiding this comment.
If this is just a reference to the file we can remove it right? We always know what the files are based on the functions: getTeamWorkloadValuesFilePath and getTeamWorkloadValuesManagedFilePath
There was a problem hiding this comment.
This is required by loadToSpec to build the files map. I am open to suggestions though.
| this.repo.users ??= [] | ||
| this.repo.versions ??= {} as Versions | ||
| this.repo.teamConfig ??= {} | ||
| this.repo.files ??= {} |
There was a problem hiding this comment.
I think it's not useful to store these file locations right? If we want to store them, these files are per workload right? So it should then be in the teamconfig
There was a problem hiding this comment.
I am trying to go for more straight forward solution where each file content in repo is reference by its file path.
In general this.repo.files is a map filePath => fileContent
There was a problem hiding this comment.
I do not understand why api is trying to rebuild the teamConfig structure.
The API should work with separate files not structures that apl-core is using.
This change anticipate that workload values are not stored with the
values |prefix.e.g.
previous
current
depends-on: linode/apl-core#2642